Skip to content

feat(dice): recognize Midas Venice as a deferred fail-closed DICE device#21

Open
boggspa wants to merge 210 commits into
mrmidi:DICEfrom
boggspa:feat/midas-venice-recognition
Open

feat(dice): recognize Midas Venice as a deferred fail-closed DICE device#21
boggspa wants to merge 210 commits into
mrmidi:DICEfrom
boggspa:feat/midas-venice-recognition

Conversation

@boggspa

@boggspa boggspa commented Jun 4, 2026

Copy link
Copy Markdown

What

Recognizes the Midas Venice (F-series) FireWire interface in the audio DeviceProfiles single-source-of-truth, so the driver identifies it in discovery/diagnostics instead of treating it as an unknown device.

Fail-closed by design

Recognition is identity-only: the audio profile reports integration mode kNone — the same "recognized but deferred" treatment DICE already gives the deferred Focusrite multistream models (SPro40 / SPro26 / Liquid 56). The factory does not instantiate a runtime protocol, so no CoreAudio endpoint is published.

This is deliberate. Standard DICE stream caps come back runtime_caps_not_ready on this hardware (0/0 channels) until a DICE EAP / current-config probing path and clock setup exist. Publishing a guessed endpoint would mislead testers and disturb hardware state — so this recognizes now, and publishes later.

IDs / provenance

Observed on real Midas Venice hardware via FFADO + Config ROM: vendor OUI 0x10c73f, model 0x000001 (FFADO driver="DICE", mixer="Generic_Dice_EAP"). The match is vendor-wide so the F16 / F24 / F32 line is recognized under one honest "Venice" name rather than guessing the variant.

Changes

  • AudioDeviceIds: add kMidasVendorId / kVeniceModelId + display names
  • Vendors/MidasAudioProfiles: identity match + kNone audio profile (mirrors the other vendor providers)
  • AudioProfileRegistry: wire the Midas provider into both lookups
  • DeviceProtocolFactory: re-export the constants for existing call sites
  • tests: Midas coverage in DeviceProtocolFactoryTests + AudioProfileRegistryTests

Testing

cmake -S tests -B build/tests_build && ctest849/849 pass (including 2 new Midas tests). No behavior change for any existing device.

The EAP probing path — the actual next step toward Midas audio — is intentionally a separate future slice.

🤖 Generated with Claude Code

mrmidi and others added 30 commits November 9, 2025 23:47
Removed unnecessary question from README.
- Logging: Introduce compact V1 AR/AT packet-flow logs to provide concise packet-flow
  visibility while reducing noisy dumps.
  - Runtime-configurable verbosity (V0-V4); V1 emits compact one-liner AR/AT packet-flow
    traces for readable packet flows.
  - Adds `LogConfig`, UI controls for logging, and UserClient APIs for runtime configuration.

- Async Transmit (AT) fixes: Correct Z-nibble and descriptor block handling.
  - Z nibble now correctly uses `TotalBlocks` (fixes block-count mismatch).
  - `PrevLastBlocks` now stores the last descriptor block count (previously inconsistent).
  - PATH2 flush now uses actual `PrevLastBlocks` values to avoid incomplete PATH2 flushes.
  - Includes updated unit tests and a validation tool to guard against regressions.
Updated README to improve clarity and structure, including status updates, architecture flow, and performance metrics.
…ntegration

- Isochronous TX: Linux-style padding workaround, HW-delta refill, zero-copy shared buffer POC
- Isochronous RX: DMA working (packet-per-buffer INPUT_LAST descriptors), shared-memory queue, cycle-time clock recovery
- CoreAudio driver: audio nub with dynamic channel count, clock synchronization PLL, HAL properties, ring buffer semantics
- DICE capability discovery and Saffire stream-mode bring-up (48kHz non-blocking packetization)
- Unified cycle-time clock recovery for TX and RX streams
- Tuned TX buffering profiles, lowered reported device latency
- Raw AV/C command tab, FCP timeout/freeze fix, Saffire mixer UI
- Oxford/Apogee protocol stubs and vendor command test scaffold
- Apogee Duet stream-mode quirk support
- Driver orchestration refactored into modules
AR DMA buffers mapped with kIOMemoryMapCacheModeInhibit require strict
natural alignment on ARM64. Block-read response payloads are only
4-byte (quadlet) aligned, but std::memcpy may emit 8-byte loads,
triggering EXC_ARM_DA_ALIGN / SIGBUS for length >= 8.

Fix: copy payload into stack buffer using quadlet-aligned reads in
RxPath::ProcessReceivedPacket before passing downstream.

Also fix a SIGSEGV race in WatchdogCoordinator::Stop() where
Cancel() dispatches an async block that dereferences a freed timer.

Fixes mrmidi#2
…ntee

Addresses Codex review on PR mrmidi#3: uint8_t[] has only 1-byte alignment
per the C++ standard, which makes downstream reinterpret_cast<uint32_t*>
technically UB. Adding alignas(4) eliminates this at zero cost.
…k sync

Isochronous subsystem refactored into layered architecture:

DMA layer (new, hardware-only, no audio semantics):
- IsochRxDmaRing: generic IR descriptor engine, replaces monolithic RX context
- IsochTxDmaRing: generic IT engine with two-cursor refill and silence ring
- IsochTxDescriptorSlab: OHCI IT descriptor slab allocator
- IsochReceiveContext relocated to Isoch/Receive/ (parallel to Transmit/)

Audio pipeline layer (new, above DMA):
- IsochAudioRxPipeline: CIP parsing, AM824 decoding, shared queue pump,
  external sync correlation
- IsochAudioTxPipeline: AM824 encoding, SYT generation, sample timing

Audio driver (refactored and expanded):
- AudioClockEngine: 48kHz clock locking against OHCI cycle timer (PID control)
- AudioIOPath: zero-copy buffer handoff and sample time correlation
- AudioDriverConfig / AudioDriverConfigPolicy: device-specific port
  provisioning and format negotiation with quirk support
- AudioControlBuilder: DriverKit control nub registration for DAC/ADC/mixer
- AudioSharedMemoryBridge: SPSC queue bridge between pipeline and IOPath
- ASFWProtocolBooleanControl: DriverKit boolean control nub (phantom, polarity)

External sync (new):
- ExternalSyncBridge: OHCI cycle timer to wallclock correlation
- ExternalSyncDiscipline48k: PID frequency/phase lock for 48kHz streams
- RxBufferProfiles: configurable RX buffer sizing profiles

TX verification and recovery (new):
- IsochTxVerifier: watchdog-driven packet loss and corruption detection
- IsochTxRecoveryController: graceful underrun and corruption recovery
- TxVerifierDecode: TX capture and decode helpers

Device protocol: Apogee Duet
- Full Apogee Duet protocol: phantom power, polarity, DSP settings, stream-
  mode quirk, hardware-backed DriverKit controls
- DeviceProtocolFactory extended for Duet instantiation
- Oxford/Apogee stub foundations

Bug fixes:
- Fixed 7 critical bugs in Focusrite Saffire PRO 24 DSP DICE implementation
- Unified cycle-time clock recovery for TX and RX streams

Swift app:
- DuetControlView/ViewModel/Models: SwiftUI panel for Duet hardware controls
- DriverConnector+Duet: userspace connector extension for Duet commands
- Raw AV/C command tab for debugging
- Independent isochronous verbosity and telemetry logging controls

C++ modernization:
- IONew()/IOSafeDeleteNULL() throughout (DriverKit memory model)
- std::make_shared<> for shared ownership (IRM, CMP clients)
- const-correctness and init-statement scoping

Test coverage (+13 files):
- IsochRxDmaRingTests, IsochTxDescriptorSlabTests
- AudioIOPathTests, AudioDriverConfigPolicyTests
- ExternalSyncBridgeTests, ExternalSyncDiscipline48kTests
- SYTGeneratorTests, TxVerifierDecodeTests
- ApogeeBooleanControlMappingTests, DeviceProtocolFactoryTests
- DuetControlViewModelTests, DuetCodecTests, AVCUnitWireParsingTests
- AudioDriverKit mock header for host-side DriverKit testing

Removed: stale architecture .md files (ACK_REFACTOR, AUDIODRIVER, AVC,
TLABEL, ASYNC_ACK_DEBUGGING)
…letion fix

## FWCommon.hpp — Bus options field layout corrected

The old `BIBFields` namespace had every bit position wrong: it read link speed,
max_rec, and generation from Bus Info Block quadlet 0 (the header) instead of
quadlet 2 (bus options). All positions were off by one or more quadlets.

Replaced with two correct namespaces:
- `ConfigROMHeaderFields` — quadlet 0: bus_info_length [31:24], crc_length [23:16], crc [15:0]
- `BusOptionsFields` — quadlet 2 per TA 1999027: irmc/cmc/isc/bmc/pmc flags,
  cyc_clk_acc [23:16], max_rec [15:12], max_ROM [9:8], generation [7:4], link_spd [2:0]

Added `BusOptionsDecoded` struct, `DecodeBusOptions()`, `EncodeBusOptions()`, and
`SetGeneration()` as constexpr helpers. Added static_assert suite verifying that
reserved bits [11:10] and [3] are disjoint from all active field masks.

## ConfigROMBuilder.cpp — Remove bad bus options mutation

The `DeriveBusInfoQuad()` helper was silently mirroring max_rec into max_ROM when
max_ROM was zero — spec-non-compliant mutation of the OHCI-provided bus options.
Replaced with a single call to `FW::SetGeneration(busOptions, generation)` which
touches only the generation bits and preserves everything else verbatim.

Also removed three local `constexpr` variables (`kDirTypeImmediate`, `kDirTypeLeaf`,
`kCycClkAccMask`) that duplicated values already in `FWCommon.hpp` or were never used.

## ConfigROMStore.cpp — Fix BIB parsing and text leaf parsing

`ParseBIB()` was reading link speed from wrong bits of quadlet 0 and ignoring
quadlets 1 and 2 entirely. Rewritten to:
- Parse quadlet 0 via `ConfigROMHeaderFields` (bus_info_length, crc_length, crc)
- Validate quadlet 1 as 0x31333934 ("1394")
- Decode quadlet 2 via `DecodeBusOptions()` for all capability/speed/generation fields
- Verify BIB CRC-16 (log-only, per IEEE 1212 §8.3)
- Root directory entry cap raised 16 → 64 (real DICE/Focusrite devices exceed 16)

`ParseTextDescriptorLeaf()` had two independent bugs:
1. Read `typeSpec` (descriptor_type + specifier_ID) from offset +2 instead of +1.
   Per IEEE 1212-2001 Figure 28: +1 is type/specifier, +2 is width/charset/lang.
2. Skipped NUL bytes instead of treating the first NUL as string terminator.

Both fixed. Width/charset/lang quadlet at +2 now explicitly validated as zero
(required for minimal ASCII form per IEEE 1212 §8.4.2.2).

## ROMReader.cpp — Fix header-first entry count extraction

In header-first autosize mode (count=0), the directory header's `entry_count`
was extracted from bits [15:0] — the CRC field. Correct field is bits [31:16].
IEEE 1212 directory header layout: [length:16][crc:16]. Added 64-entry safety cap.

## ROMScanner — Async ROM scan and double-completion fix

Major rewrite to support lazy prefix fetching for leaves and unit directories:

**Double-completion bug fixed (`completionNotified_` latch):**
`CheckAndNotifyCompletion()` is called from ~20 async callback sites. Previously it
had no guard: after the first `onScanComplete_` fired and `DrainReady()` consumed
`completedROMs_`, a queued `ScheduleAdvanceFSM()` dispatch would re-enter
`CheckAndNotifyCompletion()`, see all nodes still in terminal state with
inflightCount=0, and fire `onScanComplete_` a second time — this time with an empty
result set, causing all discovered devices to be immediately marked lost and the AV/C
transport to stall mid-FCP sequence.

Fix: `bool completionNotified_` is set as the first action in the notification path
and checked as an early-return guard on every subsequent entry. Reset only by
`Begin()`, `Abort()`, and `TriggerManualRead()`. `currentGen_` is zeroed *after*
the callback returns so the callback can still call `DrainReady(gen)`.

**`EnsurePrefix()` — lazy async ROM prefix growth:**
New private method that grows `node.partialROM.rawQuadlets` to at least
`requiredTotalQuadlets` by issuing additional `ReadRootDirQuadlets()` calls.
Used by `OnRootDirComplete` to chain async fetches for text descriptor leaves,
descriptor directories, and unit directory entries without blocking.

**`ScheduleAdvanceFSM()` — re-entrancy guard:**
All FSM advancement from within async callbacks is now routed through
`ScheduleAdvanceFSM()` (dispatches to `dispatchQueue_`). Direct `AdvanceFSM()`
calls from callbacks caused stack overflow and iterator invalidation on `nodeScans_`.

**`NodeState::ReadingDetails` added:**
New state between `ReadingRootDir` and `Complete`. A node in `ReadingDetails` has a
root dir but is still fetching leaves and unit dirs via `EnsurePrefix` chains.
`CheckAndNotifyCompletion()` requires all nodes to be in `Complete` or `Failed`.

**`ROMReader` header-first mode enabled (count=0):**
All `ReadRootDirQuadlets()` call sites now pass `maxQuadlets=0` (autosize). The
hardcoded `CalculateROMSize(bib)` formula was fragile; header-first mode reads
exactly as many quadlets as the directory header claims.

**`RootDirStartQuadlet()` / `RootDirStartBytes()`:**
New constexpr helpers replace hardcoded offset `20` / `5 quadlets`. Correct formula
is `1 + bib.busInfoLength` quadlets (BIB is not always exactly 5 quadlets).

## DiscoveryTypes.hpp — BusInfoBlock and UnitDirectory

`BusInfoBlock` fields corrected to match TA 1999027: removed wrong fields
(`linkSpeedCode`, `infoVersion`, `vendorId`), added full decoded field set.

`UnitDirectory` struct added: holds `unitSpecId`, `unitSwVersion` (both widened
from uint8_t to uint32_t — IEEE 1212 unit spec IDs are 24-bit values),
`logicalUnitNumber`, `modelId`, `modelName`.

`DeviceRecord::unitSpecId`/`unitSwVersion` widened uint8_t → uint32_t.

## DeviceRegistry.cpp — Read unit info from UnitDirectory structs

`UpsertFromROM()`, `ClassifyDevice()`, and `IsAudioCandidate()` now read unit
spec/sw version from `ConfigROM::unitDirectories` instead of raw `rootDirMinimal`
entries. Added `#if !defined(ASFW_HOST_TEST)` guard around protocol factory code.

## IsochReceiveContext — Rename ring_ → rxRing_

`Rx::IsochRxDmaRing ring_` renamed to `rxRing_` in header and all four call sites
in the .cpp. Disambiguates from the base class `DmaContextManagerBase::ring_`
(a `DescriptorRing&` reference). Eliminates the SonarCloud S2387 blocker.

## HostDriverKitStubs.hpp — Fix OSSharedPtr memory leak

The test stub for `OSSharedPtr<T>` stored a raw `T* ptr_` with no destructor.
When `ROMScanner` started storing `dispatchQueue_` as `OSSharedPtr<IODispatchQueue>`,
the stub leaked the queue object. Changed internal storage to `std::shared_ptr<T>`.
Eliminates the SonarCloud S3584 blocker.

## Tests (327 passing, +30 new)

New `ASFWConfigROMTests` executable (via `asfw_configrom_host` static lib):

- `ConfigROMBIBParseTests` — verifies TA 1999027 Annex C BIB decode: all flags,
  cyc_clk_acc=0x64, max_rec=6, max_ROM=1, generation=0, link_spd=2, GUID.
- `TextDescriptorLeafParseTests` — verifies IEEE 1212 Figure 28 text leaf parse
  (correct +1 typeSpec offset, NUL termination, empty result on non-ASCII type).
- `ROMReaderHeaderFirstTests` — verifies header-first mode reads entry_count from
  bits [31:16] (not [15:0]), and caps at 64 entries.
- `BusOptionsFieldsTests` — 15 tests covering DecodeBusOptions, EncodeBusOptions
  round-trip, SetGeneration (only touches bits [7:4], preserves reserved bits),
  and bit-position regression guards for generation [7:4], link_spd [2:0],
  max_rec [15:12].

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduce a clean audio control-plane split:

- Add AudioCoordinator + AudioNubPublisher as the single front door for nub publication and stream start/stop
- Add DiceAudioBackend (TCAT/DICE, no AV/C, no CMP/PCR)
- Add AVCAudioBackend (AV/C discovery + CMP/PCR always for audio streaming)

Isoch transport is now backend-driven:

- Refactor IsochService to be transport-only and configured via explicit IsochDuplexStartParams (GUID-keyed), removing implicit coupling to DeviceManager/CMP/AVCDiscovery
- Enforce single-device streaming at the control-plane and transport layers with an explicit TODO(ASFW-MULTIDEVICE) note

DICE runtime-truth + blocking playback:

- Parse and log TCAT general TX/RX stream-format sections (iso/start/pcm/midi/speed/labels) and derive PCM vs AM824 slot geometry
- Fix IT packetization to support am824Slots > pcmChannels (MIDI placeholder slots) and proper 48k blocking cadence
- Fix SYT generation to use the actual samples-per-packet instead of assuming 8 globally

Operational fixes:

- Add ASFWAutoStartAudioStreams gate and explicit Start/Stop streaming RPCs (remove autostart side effects from queue mapping)
- Fix stop/start restart failures by allocating IR/IT DMA rings once and reusing them across Configure(), avoiding bump-slab exhaustion (AllocateRegion would overflow on second StartDevice)

Tests:

- Extend PacketAssembler + SYTGenerator host tests for blocking/non-blocking sizing and MIDI slot labeling
Refactor the ROM explorer model + UI:

- Restructure ROM parsing/model types
- Improve ROM cache handling
- Update view model + SwiftUI views for the new model

(No driver changes in this commit.)
Retain and map shared isoch queue memory inside IsochService instead of relying on transient local mappings, and treat null queue wiring as an explicit detach for both RX and TX audio pipelines. This prevents stale shared-queue pointers from surviving teardown and addresses the watchdog-side TX verifier crash when the backing mapping disappears.

Add provider termination notifications in the DriverKit service and quiesce immediately on provider termination (set stopping, stop watchdog, disable interrupts, detach hardware) so late timer/interrupt paths do not issue MMIO after Thunderbolt/PCIe hot-unplug. Stop() and teardown/reset paths now also cancel the notification source and detach hardware earlier.

Update AV/C and DICE audio start paths plus duplex isoch startup to pass retained queue memory descriptors into IsochService, with explicit ownership and early-return cleanup. Host test DriverKit stubs were extended to support the new OSSharedPtr reset overloads used by this path.

Include Saffire UI/control-path updates: resolve the Saffire target from discovery (GUID/node ID) and route TCAT application reads/writes and software notices to the discovered destination node instead of assuming node 0.

Known issue: attempting Saffire discovery and then hot-unplugging the device may still trigger a kernel panic and system reboot on Apple silicon. This commit narrows the MMIO-after-unplug window, but the unplug race is not fully eliminated yet.
Adjust AudioDriverConfigPolicy tests to match the revised max-supported-channel behavior: when directional counts are absent, ClampAudioDriverChannels now inherits from aggregate channelCount without clamping. Add a separate test that still verifies explicit directional counts are clamped to the provided maximum.
SonarQube CRITICAL cpp:S5025 fired on IsochReceiveContext.cpp because
`new (std::nothrow)` was assigned to a raw pointer before being wrapped in
OSSharedPtr. Even with an immediate wrap, SonarQube flags the raw-pointer
assignment itself.

Introduce `ASFWDriver/Common/DriverKitUtils.hpp` with a
`MakeOSObject<T>(Args&&...)` factory template that centralises the
unavoidable DriverKit allocation pattern:

  - Allocates with `new (std::nothrow)` (single NOSONAR suppression here)
  - Null-checks the result
  - Immediately transfers ownership into `OSSharedPtr<T>(raw, OSNoRetain)`
  - Returns an empty OSSharedPtr on allocation failure
  - `static_assert(std::is_base_of_v<OSObject, T>)` enforces correct usage
    at compile time
  - `[[nodiscard]]` prevents silent discard; `noexcept` since placement-new
    never throws and DriverKit disables exceptions

Update `IsochReceiveContext::Create()` to use the new helper:

  Before:
    IsochReceiveContext* rawCtx = new (std::nothrow) IsochReceiveContext();
    if (!rawCtx) return nullptr;
    rawCtx->hardware_ = hw;
    rawCtx->dmaMemory_ = std::move(dmaMemory);
    if (!rawCtx->init()) {
        rawCtx->release();   // manual, easy to miss on new paths
        return nullptr;
    }
    return OSSharedPtr<IsochReceiveContext>(rawCtx, OSNoRetain);

  After:
    auto ctx = ASFW::Common::MakeOSObject<IsochReceiveContext>();
    if (!ctx) return nullptr;
    ctx->hardware_ = hw;
    ctx->dmaMemory_ = std::move(dmaMemory);
    if (!ctx->init()) return nullptr;  // OSSharedPtr destructor calls release()
    return ctx;

The manual `rawCtx->release()` on the init-failure path is eliminated —
the OSSharedPtr destructor handles it automatically. No change to the
function signature, return type, or caller behaviour.

MakeOSObject is variadic and reusable for any other OSObject factory in
the driver (e.g. ASFWProtocolBooleanControl::Create()). All 306 C++ unit
tests pass.
strncpy does not guarantee NUL-termination when the source is exactly as
long as the destination buffer, requiring an explicit
`buf[sizeof(buf)-1] = '\0'` guard after every call — a pattern that is
easy to omit on new code paths and was already inconsistent across the
codebase (ControllerMetrics::Reset() never set the sentinel).

Replace every occurrence with strlcpy, which always NUL-terminates and
takes the full buffer size rather than size-1, making the intent clear
and eliminating the manual sentinel writes.

Files updated:
  - ASFWDriver/Debug/BusResetPacketCapture.cpp
  - ASFWDriver/Diagnostics/ControllerMetrics.cpp
  - ASFWDriver/Diagnostics/StatusPublisher.cpp
  - ASFWDriver/UserClient/Handlers/BusResetHandler.cpp
  - ASFWDriver/UserClient/Handlers/StatusHandler.cpp
  - ASFWDriver/UserClient/Handlers/TopologyHandler.cpp

DriverVersionInfo.hpp additionally replaces the manual default constructor
(which called memset) with C++ default member initialisers on each field.
The memset-based constructor was redundant: aggregate initialisation with
`{}` already zero-initialises all members, and the explicit constructor
was only needed to paper over the missing in-class initialisers. The
change also migrates the six strlcpy calls in the static factory method.
…ead rule-of-5

The ROMScanner constructor was refactored in 314a182 to accept a
ROMScannerParams struct, but the call-site in ASFWDriver::Start() was
not updated, leaving the driver calling the old three-argument overload.
Pass a default-constructed ROMScannerParams{} to match the new signature.

The test helper struct PendingRead stores an
InterfaceCompletionCallback (a std::function), which is non-trivially
copyable. Storing PendingRead values in a std::vector<PendingRead> and
calling operations like push_back or emplace_back can trigger implicit
copy/move construction. Without explicit rule-of-5 declarations the
compiler may suppress the move constructor when a non-trivial copy
constructor is present, causing unnecessary copies or outright
compilation errors depending on the C++ version and optimisation level.
Explicitly default all five special members on PendingRead to restore
the expected copy+move semantics.
… capture (S1048)

Two bugs in ObserverGuard<T>:

1. Destructor could terminate on throwing Unregister (S1048)
   The destructor called unregister_() — a std::function storing a
   virtual Unregister* call — with no exception guard. The destructor
   is already marked noexcept, so any exception thrown by the callee
   would invoke std::terminate rather than propagate. Added a
   try/catch(...) to swallow any exception explicitly, making the
   intent clear and satisfying SonarQube cpp:S1048.

2. IUnitObserver branch captured registry by reference
   The lambda stored in unregister_ captured `registry` by reference
   (i.e. a reference-to-reference bound into a std::function that
   outlives the constructor). If the registry was destroyed before the
   ObserverGuard, the destructor would call through a dangling
   reference — UB with no diagnostic.

   The IDeviceObserver branch already handled this correctly by taking
   &registry as a typed pointer and capturing that pointer by value.
   Apply the same pattern to IUnitObserver: cast to IUnitRegistry*
   and capture the pointer, matching the existing convention.
   static_cast is correct here (dynamic_cast is only needed in the
   IDeviceObserver branch where RegistryType may not implement
   IDeviceManager).
The branchless CRC32 inner loop used `-(crc & 1u)` to produce an
all-zeros or all-ones uint32_t mask. While well-defined (unsigned
wraparound), SonarQube cpp:S876 flags unary minus on unsigned types as
error-prone — the result is a large positive value, not a negative one,
which is surprising to readers.

Replace with an explicit ternary that makes the all-zeros / all-ones
intent clear without relying on unsigned wrap semantics.
…lication

Group A (19 issues): Add NOSONAR(cpp:S3923) to if/else blocks where both
branches contain only ASFW_LOG* calls with different format strings.
os_log requires compile-time literal format strings, so branches cannot
be merged with a ternary — the structural identity is a false positive.

Files: AsyncSubsystem.cpp, Tracking.hpp, BusResetCoordinator.cpp,
SelfIDCapture.cpp, DeviceRegistry.cpp, IsochAudioTxPipeline.cpp,
LogConfig.cpp, AVCUnit.cpp, DICETransaction.cpp, SPro24DspProtocol.cpp,
IsochHandler.cpp

Group B (1 real fix): MusicSubunit.cpp — the first and third branches of
the plug-direction if/else both assigned kInput. Extract it as the default
before the conditional; keep the inner if/else only to override kOutput
and log the out-of-range warning. No behaviour change.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace raw IOBufferMemoryDescriptor* pointers with OSSharedPtr<> in
StartIsochReceive, StartIsochTransmit (ASFWDriver.cpp) and
StartStreaming (AVCAudioBackend.cpp, DiceAudioBackend.cpp).

Use .attach() to receive the pointer from CopyRxQueueMemory /
CopyTransmitQueueMemory and .detach() to transfer ownership into
StartReceive / StartTransmit. This eliminates all manual ->release()
calls on error paths and ensures the buffer is released automatically
if an early-return path is added in future.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mrmidi and others added 28 commits June 2, 2026 06:51
- Cycle Policy: Implement local cycle master clearing when the node is not root to prevent stale state propagation.
- CSR Verification: Do not fail CSR contract verification solely due to obsolete/stale SPEED_MAP generation.
- IRM Diagnostics: Document autonomous OHCI CSR handling for IRM registers and mark them as not software-observable in diagnostics.
- Diagnostics UI: Update diagnostics text formatting to support AT Tx retry parsing, local clear counts, and revised CSR contract states.
- Tests: Add unit tests for clearing local cycle master and verify contract verifier rules.
…Profiles

Introduce a header-only, family-based DeviceProfiles metadata layer and make it
the single source of truth for known-audio-device identity and integration mode,
so the Discovery data path no longer reaches into the audio protocol factory for
classification.

- Add ASFWDriver/DeviceProfiles/{Common,Audio,Audio/Vendors}: constexpr providers
  (Focusrite/Apogee/Alesis) plus an AudioProfileRegistry aggregator. Metadata only
  - no runtime IDeviceProtocol construction, no allocation, no link dependency.
- DeviceProtocolFactory now delegates its identity/integration-mode lookups to
  DeviceProfiles and re-exports the id/name constants as aliases, so there is a
  single device table. Create() and the legacy KnownIdentity surface are unchanged.
- DeviceRegistry uses AudioProfileRegistry for name enrichment, Focusrite GUID
  inference, and the integration-mode -> kind/isAudioCandidate classification.
  ClassifyDevice/IsAudioCandidate and runtime protocol creation are untouched
  (DeviceRecord::protocol and creation move to the Audio layer in a follow-up).
- Add AudioProfileRegistryTests; DeviceProtocolFactoryTests stays green via the
  delegation, confirming behavior is preserved.

Phase A of removing audio-runtime leakage from Discovery. 808/808 host tests
pass; Xcode dext build succeeds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ery (Phase B)

Discovery is now metadata-only. The device-specific runtime protocol objects
(formerly owned by DeviceRegistry via DeviceRecord::protocol and constructed in
MaybeCreateKnownProtocol) move into a new Audio-layer owner.

- Add Audio/AudioRuntimeRegistry (guid -> shared_ptr<IDeviceProtocol>): a
  control-plane registry with a small IOLock that hands back shared_ptr copies,
  closing the prior raw-borrowed-pointer use-after-free seam. EnsureForDevice
  wraps DeviceProtocolFactory::Create + Initialize; Insert supports external/test
  injection; the hot audio packet path never touches it.
- Own it in ControllerCore::Dependencies, constructed in DriverContext::EnsureDeps
  before AudioCoordinator and ControllerCore. Exposed via
  ControllerCore::GetAudioRuntimeRegistry() (header forward-decl; .cpp-only
  include keeps the new Driver->Audio edge out of the public header).
- Trigger creation from the controller discovery path (ControllerCoreDiscovery),
  where bus + IRM are already in scope -- no SetBusAccess injection needed.
- Migrate the four runtime consumers off record->protocol to FindShared(guid):
  ASFWAudioNub (binding gains a protocolOwner shared_ptr held across boolean-control
  calls), ASFWDriver wire-format, DiceAudioBackend, DiceDuplexRestartCoordinator
  (RequireDiceRecord hands back an outHold; ResolveDuplexChannelsForRecord takes the
  protocol).
- De-leak Discovery: UpsertFromROM is now 2-arg metadata-only; remove
  MaybeCreateKnownProtocol, the DeviceProtocolFactory include, DeviceRecord::protocol,
  and the ASFW::Audio forward decl.

813/813 host tests pass; full dext build green. Discovery has no remaining
Protocols/Audio / IDeviceProtocol / DeviceProtocolFactory / ASFW::Audio references.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ServiceContext::Reset() destroyed hardware, async, and IRM dependencies but
never reset deps.audioRuntimeRegistry. Since controller.reset() drops the
controller's copy of that shared_ptr, the deps copy became the last owner and
kept live IDeviceProtocol instances alive past bus/hardware/IRM teardown -- the
protocols hold busOps/busInfo/irmClient (DeviceProtocolFactory::Create), so their
destructors could touch freed services (and a stop->start cycle would reuse a
registry full of stale protocols).

Reset deps.audioRuntimeRegistry right after audioCoordinator.reset() and before
the bus/hardware/IRM teardown, so the protocol destructors run while the services
they reference are still alive.

Also document that runtime-protocol creation is orchestrator-serialized (single
Default queue), and soften an over-stated "permanent home" comment.

Full dext build green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ove logging

- Reorder the duplex startup sequence in DiceDuplexRestartCoordinator to start host receive after arming the device transmitter.
- Cache the total channel and slot capacity for DICE runtime capabilities instead of only the active ones (TotalPcmChannels() and TotalAm824Slots() instead of ActivePcmChannels() and ActiveAm824Slots()).
- Update unit tests (DICETcatProtocolTests) to match capabilities caching changes.
- Add error logging in DICEDuplexBringupController::DoPollSourceLock when the clock fails to lock.
…T offset wrapping

- Move startup alignment latching in AudioIOPath to HandleIOOperation so playback-only (output-only) sessions correctly anchor timing without input pulls.
- Remove pre-start seed gate validation loop in IsochService::StartTransmit.
- Bootstrap the transmit pipeline with nominal phase (0x0000) when the RX external sync seed is stale or unavailable instead of failing transmit start, and update tests to verify.
- Correctly handle out-of-range/overflow tick indexes (greater than 3072 ticks per cycle) during SYT tick index decoding.
- Refactor SYTGenerator::computeDataSYT to calculate relative phase offset and properly wrap across the 16-cycle OHCI domain.
- Add rate-limited logging to reduce zts/fallback noise, warn when alignment timing is not ready, and log details of the first 16 audio callbacks.
Perform a project-wide architectural cleanup and unification of endianness handling.

- Relocate audio-related source files to their correct ownership domains:
  - ADK-facing driver/nub/controls to Audio/DriverKit.
  - Wire-format (AM824/CIP/AMDTP) helpers to AudioWire.
  - Legacy audio pipelines to AudioEngine/LegacyIsoch.
  - Protocol backends to Protocols/Audio/Backends.
- Unify endianness conversion to use standard DriverKit/macOS OSSwap* macros:
  - Replace custom SwapBigToHost, byteSwap32, ToBE*, FromBE* helpers project-wide.
  - Ensure <DriverKit/IOLib.h> inclusion where necessary.
  - Delete redundant endianness helper definitions.
- Merge redundant IsochTypes.hpp headers into Isoch/Core/IsochTypes.hpp.
- Cleanup speculative comments and "chain of thought" notes in CIPHeader.hpp.
- Update tests/CMakeLists.txt and test source files to match the new tree structure
  and endianness primitives.

Verified that the driver builds successfully and all 816 tests pass.
…, and integrate hardware path

- Rename DirectTxScratchEncoder.hpp to DirectTxPacketEncoder.hpp and generalize scratch-specific types to general direct TX packet layout helpers.
- Introduce TxAudioPacketWriter to write audio streams and CIP headers directly to target hardware/DMA payload buffers without intermediate scratch copies.
- Integrate the direct TX path into IsochAudioTxPipeline::InjectNearHw via TryWriteDirectTxPacket, guarded by a kEnableDirectTxHardwarePath toggle and direct runtime binding validity checks.
- Add direct TX runtime bindings (DirectTxRuntimeBinding) and expose them via IsochTransmitContext to couple the audio engine skeleton to the transmission pipeline.
- Implement telemetry counters in IsochAudioTxPipeline for tracking direct transmission packets, underrun silence, invalid packets, and legacy fallbacks.
- Add TxAudioPacketWriterTests to verify PCM encoding, silence generation on underrun, wrap-around buffering, MIDI slot padding, and safety boundaries.
Replace the callback-based DirectTxRuntimeBinding (writePacket/
publishConsumedEndFrame function pointers into the ADK object graph) with
a plain, RT-safe view of the host->device output stream memory and the
shared AudioTransportControlBlock. The isoch TX hot path now owns its own
DirectOutputReader/TxAudioPacketWriter over those handles and never calls
back into ASFWAudioDriver, mirroring the existing zero-copy precedent.

- IsochAudioTxPipeline: memory/control DirectTxRuntimeBinding; local
  DirectOutputReader rebuilt in SetDirectTxRuntimeBinding; TryWriteDirectTxPacket
  drives TxAudioPacketWriter; consumed cursor published into
  control->outputConsumedEndFrame (release store).
- InitializeDirectOutputCursor anchors the output cursor at WriteEnd minus a
  safety lead (max(64, 3x framesPerPacket)) on the first DATA injection, and
  defers to the legacy path until the client has published a buffer.
- Thread directOutput memory/control + sampleRate + directTxEnabled through
  IsochDuplexStartParams, IsochService::StartTransmit (default-disabled args),
  and StartDuplex. Existing callers stay disabled.
- Startup logging at wire time and first cursor init.
- Tests target gains TxAudioPacketWriter.cpp + DirectTxProbe.cpp.

Hardware path remains gated off (kEnableDirectTxHardwarePath=false); 859/859
host tests + dext build green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…AVC path only; DICE late-bind pending)

Source the direct TX output view from the audio side and turn on the
compile-time gate. Both ASFWDriver and ASFWAudioDriver share one dext
server process (identical IOUserServerName), so the ADK output buffer
address and the AudioTransportControlBlock are handed across as raw
pointers (control block fields are atomic = the RT-safe channel).

- ASFWAudioNub: SetDirectAudioBinding / ClearDirectAudioBinding /
  GetDirectAudioBinding (LOCALONLY) + IVars holding the output-buffer view
  and control-block pointer.
- ASFWAudioDriver: register the binding with the nub after
  BindDirectAudioSkeleton (using the graph's already-computed output base /
  frame capacity / channels / control / rate); clear it in Stop() while the
  provider is still valid.
- ASFWDriver::StartIsochTransmit: fetch the binding from the nub and pass it
  to StartTransmit (directTxEnabled = present).
- kEnableDirectTxHardwarePath = true.

KNOWN-INCOMPLETE: this sources the binding only on the AVC
StartIsochTransmit path. DICE/Saffire brings TX up via StartDuplex /
IsochDuplexStartParams, which is not yet populated, and the at-start fetch
is too early regardless (IT starts before the ADK driver registers its
buffer). Next step is pull-based late-binding via a host-test-safe
DirectBindingSource seam so the running IT context self-arms when the
binding appears, on both AVC and DICE paths. dext build + 859/859 host
tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Fix compilation errors in ASFWAudioDriver by removing legacy shared queue mappings.
- Clean up AudioDriverRuntimeState by deleting obsolete ioMetrics and encodingMetrics.
- Delete unused LegacyBridge (AudioClockEngine, AudioIOPath, AudioSharedMemoryBridge).
- Rename LegacyIsoch to DirectIsoch.
- Remove TxSharedQueue and RxSharedQueue dependencies across all isochronous contexts.
- Silence compiler warnings (ignored nodiscard returns, unused variables, narrowing conversions) in DICE protocols and Isoch contexts.
- Add RawPcm24In32 shared codec for direct memory packetization.
- Update tests/CMakeLists.txt to remove deleted legacy targets and fix test compilation.

Verified driver builds successfully and all 859 remaining tests pass cleanly.
- Remove orphaned function declarations from ASFWAudioNub.iig.
- Delete unused legacy RPC implementations and TxSharedQueue include from ASFWAudioNub.cpp.
- Remove leftover TxSharedQueueSPSC members from AudioDriverSharedMemoryState.
- Fix missing contextIndex argument in IsochService::StartReceive.
- Remove obsolete GetStreamProcessor() calls from WatchdogCoordinator and IsochHandler.
- Remove unused kEnableZeroCopyOutputPath from ASFWAudioDriver.
- Delete obsolete IsochTransmitContextTests from CMakeLists.txt.

Build and tests are now clean and warning-free.
Add Midas Venice (F-series) identity to the audio DeviceProfiles single
source of truth so discovery names the device instead of treating it as
unknown. Recognition is identity-only and fail-closed: the audio profile
reports integration mode kNone (recognized-but-deferred, like the deferred
Focusrite multistream models) and the factory does not instantiate a
runtime protocol, so no CoreAudio endpoint is published.

Standard DICE stream caps return runtime_caps_not_ready on this hardware
until the EAP / current-config probing path and clock setup exist;
publishing a guessed 32x32 endpoint would mislead testers and disturb
hardware state. Recognize now, publish later.

IDs observed on real Midas Venice hardware via FFADO + Config ROM
(vendor OUI 0x10c73f, model 0x000001, FFADO "Generic_Dice_EAP"). The match
is vendor-wide so the F16/F24/F32 line is recognized under one honest
"Venice" name rather than guessing the variant.

- AudioDeviceIds: add kMidasVendorId / kVeniceModelId + display names
- Vendors/MidasAudioProfiles: identity match + kNone audio profile
- AudioProfileRegistry: wire the Midas provider into both lookups
- DeviceProtocolFactory: re-export the constants for existing call sites
- tests: Midas coverage in DeviceProtocolFactory + AudioProfileRegistry

All 849 host tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mrmidi mrmidi force-pushed the DICE branch 2 times, most recently from 1ae1cce to e6753ed Compare June 12, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants